FIX: uum 138143 button press points for stickcontrol and vector2 add functionality#2411
Conversation
…ld used for stick controls and Vector2. Test
…at a user declared as an interaction.
|
The diff contains approximately 325,668 tokens (~977,005 characters), which exceeds the maximum limit of 150,000 tokens. Please consider splitting this PR into smaller, focused changes. 🤖 Helpful? 👍/👎 |
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## develop #2411 +/- ##
===========================================
+ Coverage 78.13% 78.91% +0.78%
===========================================
Files 483 756 +273
Lines 98770 139229 +40459
===========================================
+ Hits 77169 109867 +32698
- Misses 21601 29362 +7761 Flags with carried forward coverage won't be shown. Click here to find out more.
|
…-stickcontrol-and-vector2-add-functionality # Conflicts: # Packages/com.unity.inputsystem/InputSystem/Runtime/Controls/InputControlExtensions.cs
ekcoh
left a comment
There was a problem hiding this comment.
@Darren-Kelly-Unity Would it be possible to get rid of all the whitespace diffs on this PR? It would be much easier to review if this was corrected.
…-stickcontrol-and-vector2-add-functionality
…-stickcontrol-and-vector2-add-functionality
… it was implicitly implemented.
ekcoh
left a comment
There was a problem hiding this comment.
Reviewed, I had quite a lot of questions and some remaining from last review so leaving this as comment-only for now.
| /// <summary> | ||
| /// Effective press threshold: <see cref="pressPoint"/> when set, otherwise the global default. | ||
| /// </summary> | ||
| float pressPointOrDefault { get; } |
There was a problem hiding this comment.
Why do we need two? Would it be better to have one, e.g. pressPoint that returns default if not having a layout-configured press threshold? Since both go through an interface anyway I wouldn't expect it to be an optimisation?
There was a problem hiding this comment.
This is just following the format that was already there for now, not sure about removing one as it was publicly available before.
There was a problem hiding this comment.
If it was public we cannot remove it now (unless it fits a scheme to deprecate), but I am afraid two variants creates confusion around the interface contract that a user should implement.
There was a problem hiding this comment.
But if the new interface only implemented the previous implicit function:
pressPointOrDefault wouldn't this still work since it takes the same value if available and otherwise fall back?
There was a problem hiding this comment.
Since if (control is IActuationPressPoint actuation) is the new condition also covering Button?
There was a problem hiding this comment.
I can try that and see if it works, you may be right yes!
…ctor2Control.cs Co-authored-by: Håkan Sidenvall <hakan.sidenvall@unity3d.com>
…ctor2Control.cs Co-authored-by: Håkan Sidenvall <hakan.sidenvall@unity3d.com>
…ves." This reverts commit bce3776.
…es test coverage shown." This reverts commit 1243bc4.
…t improves test coverage shown."" This reverts commit db81e01.
…see if it improves test coverage shown.""" This reverts commit 8ff8746.
…-stickcontrol-and-vector2-add-functionality
…-stickcontrol-and-vector2-add-functionality
…-stickcontrol-and-vector2-add-functionality
Purpose of this PR
Fixes UUM-138143 —
InputAction.IsPressed()/ in-progress style checks could fire at the wrong effective magnitude for Vector2 / stick bindings when developers setpressPointon the control or onPressInteraction, instead of honoring those thresholds consistently.Introduces
IActuationPressPoint, implements it onButtonControl,Vector2Control, and (viaVector2Control)StickControl, and threadspressPoint/pressPointOrDefaultthroughInputAction,InputActionState, andInputControlExtensionsso press-style polling matches the documented interaction defaults. AddsActuationPressPointTestsand refreshesRespondingToActions.md.Release Notes
InputAction.IsPressed()(and related press / in-progress behavior) for actions bound toVector2ControlandStickControlnow respects per-controlpressPointand bindingPressInteraction.pressPoint, aligning withButtonControland fixing incorrect early triggers when rewriting default interaction thresholds.Functional Testing status
ActuationPressPointTestsand updates inCoreTests_Actions/CoreTests_Controlscover press-point actuation paths.IsPressedbefore processed magnitude reaches the configured press threshold for vector / stick cases.Performance Testing Status
I have added performance testing for the hot path methods that Hakan flagged and I don't see any major issues when comparing. There is a slight increase in cost of performance of about 0.1ms for 1000 iterations on the GetActuationPressThreshold() method that was asked to be checked.
No new per-frame allocations identified in scope.
Overall Product Risks
Technical Risk: 2 — Touches
InputActionStateand widely used press APIs; edge cases around magnitude, deadzones, and interactions need regression confidence.Halo Effect: 2 — Any title using
IsPressed/WasPressedThisFrameon Vector2 / stick actions may see timing changes until bindings are tuned; behavior is intended to match documentedpressPointsemantics.Class diagram
Git range analyzed:
origin/develop...HEAD(4 commits).Mermaid class diagram (GitHub)
classDiagram direction TB class IActuationPressPoint { <<interface>> +pressPoint +pressPointOrDefault } class ButtonControl { +pressPoint +pressPointOrDefault } class Vector2Control { +pressPoint +pressPointOrDefault } class StickControl { } class InputAction { +IsPressed() +WasPressedThisFrame() +WasReleasedThisFrame() } class InputActionState { } class InputControlExtensions { <<static>> } IActuationPressPoint <|.. ButtonControl IActuationPressPoint <|.. Vector2Control Vector2Control <|-- StickControl InputAction --> InputActionState : uses InputControlExtensions ..> IActuationPressPoint : pressPointOrDefaultDocumentation Impact
Changes analyzed:
origin/develop...HEAD(4 commits).User-facing: New public
IActuationPressPoint;InputActionpress-helper remarks/behavior;ButtonControl,Vector2Control,StickControldocs aroundpressPoint;RespondingToActions.mdupdated for polling vspressPoint.Documentation updates in this PR
RespondingToActions.md— ClarifiesIsPressed/ frame helpers vs interactions andpressPoint; review wording and cross-refs after merge.Follow-up (optional)
IActuationPressPointcallout in a control reference page, add a short subsection and link from Responding to Actions.Jira (fetched via MCP): Bug, In Progress, TBD priority, assignee Darren Kelly, label Input-Actions. Ticket summary: InputAction.IsPressed() and IsInProgress() triggers on wrong values when rewriting the default values of Vector2 interactions.